Skip to content

feat(sync): rsync-over-ssh NAS transport - #31

Open
Xander-git wants to merge 21 commits into
mainfrom
feature/rsync-ssh-transport
Open

feat(sync): rsync-over-ssh NAS transport#31
Xander-git wants to merge 21 commits into
mainfrom
feature/rsync-ssh-transport

Conversation

@Xander-git

Copy link
Copy Markdown
Collaborator

Summary

Adds a second NAS transport — rsync over ssh — selected per instance by a new nas.transport config field (rclone default, rsync_ssh). Cluster machines can only reach the Synology NAS via the allowlisted rsync --server ssh channel (outbound SMB blocked, rclone-SFTP locked by IT, no remote command exec, no interactive login); this branch makes those instances first-class sync clients while lab PCs keep rclone-over-SMB untouched.

  • Design spec: docs/superpowers/specs/2026-06-10-rsync-ssh-nas-transport-design.md (all open questions resolved; implementation amendments recorded in the status header)
  • Implementation plan: docs/superpowers/plans/2026-06-10-rsync-ssh-nas-transport.md
  • Operator walkthrough: docs/setup/rsync-ssh-setup.md (keypair, ssh-keyscan host-key provisioning — needs no login permission — cluster config block)

How it works

  • New RsyncSshDriver (sync/transports/rsync_ssh.py) behind a shared NasTransportDriver protocol + build_nas_driver factory. Targets reuse the existing <remote>:<path> string (user@host:/path is valid rsync syntax), so the queue state machine, reconcile loop, and cleanup gate are untouched.
  • push = rsync -rt (mtime preservation contract) · manifest = --list-only parsed into RemoteManifest (timestamp-anchored regex; spaces/escapes/comma-grouped sizes handled) · pre-deletion verify = --checksum dry-run itemize (remote-side hashing rides the same allowlisted channel) · probe = non-recursive listing (free-space readout degrades by design; no remote exec exists).
  • ssh is key-auth only, BatchMode=yes, no StrictHostKeyChecking relaxation; host keys pre-provisioned via ssh-keyscan.
  • Stage-mode equipment always gets RcloneDriver regardless of nas.transport (staging targets are rclone named remotes; the hidden staging backend keeps working) — pinned by a regression test.
  • Setup gate in rsync mode never spawns rclone (static predicate); Settings shows a transport row + transport-aware badges.
  • Hardening: every rsync argv passes -- before positionals; config rejects leading-dash user/host; ssh auth-failure marker anchored on permission denied (publickey so a remote per-file Permission denied (13) stays retryable; listing parser warns loudly on -h-formatted sizes instead of silently dropping files.

Both transports verified

Path Evidence
rclone (existing) Pre-existing suites pass unchanged (sync 215, tray+paths+api 481, ui 665+19s, e2e 63+11s) and the live docker integration test_nas_sync.py passed 8/8 against the rebuilt sshd container
rsync_ssh (new) 33 driver unit tests + stub round-trip, plus the live docker characterization suite 4/4 — including the timezone round-trip with the container pinned to America/New_York, which empirically confirmed --list-only timestamps are client-formatted

Docker fixture grows a key-only rsync user (no SFTP subsystem — Synology posture) beside the untouched password-SFTP user.

Notes for reviewers

  • CheckResult.errors is always () in rsync mode (itemize has no per-file error line; failures raise as classified TransportError) — documented contract difference.
  • RsyncSshDriver.ssh_extra_opts is a documented test-only seam (integration fixture's throwaway known_hosts); production config does not expose it.
  • Pre-existing, untouched: test_nas_client_stability.py fails in isolation on a circular import that exists on main too (passes in the full suite run).

🤖 Generated with Claude Code

Xander-git and others added 21 commits June 10, 2026 01:22
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…n test

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ode rclone bypass

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…arg default

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…n rsync transport

Argument-injection hardening from automated security review: every rsync
argv now passes `--` before its positional local/remote arguments so a
malformed config value beginning with `-` can never be parsed as a flag,
and NasConfig rejects rsync-mode remotes whose user or host segment
starts with `-`.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- Add rsync to the SFTP container Dockerfile (Debian rsync 3.2.7)
- Provision a key-auth rsync user (rsyncuser) in entrypoint.sh:
  generates ed25519 keypair into bind-mounted keys/ dir on first boot,
  installs pubkey as authorized_keys, Match User block disables SFTP
  for that user while allowing rsync --server (Synology posture)
- Set TZ=America/New_York on the nas-sftp service in docker-compose.yml
  so the container's timezone differs from the client's; this is the
  timezone characterization condition for the test suite
- Drop --no-h from RsyncSshDriver's --list-only commands: the flag is
  GNU rsync-specific and fails on macOS openrsync (the client); the
  parser already handles comma-grouped sizes so --no-h was never needed
- Add ssh_extra_opts test seam to RsyncSshDriver (keyword-only tuple,
  default empty) for injecting UserKnownHostsFile/StrictHostKeyChecking
  in integration tests without mutating developer's ~/.ssh/known_hosts;
  production config never sets it; unit test confirms argv rendering
- Create tests/integration/test_rsync_ssh_characterization.py with
  docker-gated fixtures and 4 characterization tests:
  test_push_list_reconcile_roundtrip (TZ verdict: PASSED — timestamps
  are CLIENT-TZ formatted, spec assumption confirmed),
  test_check_differ_and_missing, test_list_only_on_missing_run_dir_raises_network,
  test_about_ok_and_missing_base_root
- Document new user/keys/TZ in tests/docker/README.md

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- Create docs/setup/rsync-ssh-setup.md: keypair generation (ed25519,
  empty passphrase), installing pubkey on Synology service account,
  host-key pre-provisioning via ssh-keyscan with explicit note that no
  login permission is needed (host-key exchange precedes auth), out-of-band
  fingerprint verification against DSM, recording rsync --version in the
  runbook, config.yaml block (rsync_ssh example from spec), degraded
  Test-connection note, and BatchMode failure-modes table
- Add cross-link from docs/setup/rclone-remote-setup.md to rsync-ssh-setup.md
- docs/design_specs/design_spec_sections/09_Configuration_File.md: add
  nas.transport / ssh_port / ssh_identity_file fields to the nas: block
  with inline comments
- docs/design_specs/design_spec_sections/07_Sync_and_Database_Integration.md:
  add §7.1.3 transport matrix (rclone vs rsync_ssh) + verify-authority-differs-
  by-transport paragraph; update §7.1.3 header from "sole transport" to
  multi-transport framing
- docs/design_specs/design_spec_sections/04_Backend_Architecture.md: add
  sync/transports/ subtree to the package layout; add NasTransportDriver
  Protocol + build_nas_driver factory paragraph
- README.md: add one-line rsync-over-ssh transport mention + link to
  rsync-ssh-setup.md in the NAS sync setup section

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…, exact stub dry-run match

Final-review findings: (1) the bare 'permission denied' auth marker
would terminally fail a job on a remote per-file filesystem error
('Permission denied (13)'); anchor on ssh's parenthesized auth-method
form instead. (2) An '-h'-formatted listing size silently dropped the
file from the manifest, re-queuing it forever — entry-shaped lines with
unparseable sizes now log a warning. (3) stub_rsync's dry-run detection
substring-matched 'n' in any flag; exact-match the driver's spellings.
The errors-always-empty CheckResult contract difference is now
documented in _synthesize_check.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant